Skip to content

fix: fix wild pointer issue when screen switching #1450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
18202781743 wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: fix wild pointer issue when screen switching #1450

18202781743 wants to merge 1 commit into from
+3 −2

Conversation

@18202781743
Copy link
Contributor

@18202781743 18202781743 commented Feb 12, 2026 •
edited by sourcery-ai bot
Loading

  1. Changed QScreen* to QPointer for m_dockScreen member
    variable
  2. This prevents dangling pointer issues when screens are removed or
    changed
  3. QPointer automatically becomes null when the QScreen object is
    deleted
  4. Added include for QPointer header file
  5. Updated copyright year range from 2023 to 2023-2026

Influence:

  1. Test dock behavior when switching between different screen
    configurations
  2. Verify dock doesn't crash when monitors are disconnected or
    reconnected
  3. Test multi-monitor setup with dock moving between screens
  4. Verify screen-related dock properties update correctly after screen
    changes
  5. Test system behavior during display configuration changes

fix: 修复屏幕切换时的野指针问题

  1. 将 m_dockScreen 成员变量从 QScreen* 改为 QPointer
  2. 防止屏幕被移除或更改时出现悬空指针问题
  3. QPointer 在 QScreen 对象被删除时会自动变为空指针
  4. 添加了 QPointer 头文件包含
  5. 更新了版权年份范围从 2023 到 2023-2026

Influence:

  1. 测试在不同屏幕配置切换时的停靠栏行为
  2. 验证断开或重新连接显示器时停靠栏不会崩溃
  3. 测试多显示器设置中停靠栏在屏幕间移动的情况
  4. 验证屏幕更改后停靠栏的屏幕相关属性正确更新
  5. 测试显示配置更改时的系统行为

Summary by Sourcery

Prevent dock panel crashes due to invalid screen references when displays are removed or changed.

Bug Fixes:

  • Guard dock panel screen tracking against dangling pointer issues during screen configuration changes.

Enhancements:

  • Use a safe Qt smart pointer type for the dock panel's associated screen reference to handle dynamic screen lifecycles more robustly.

Chores:

  • Update dock panel header copyright years to 2023–2026.

@18202781743
fix: fix wild pointer issue when screen switching
a00e427 
1. Changed QScreen* to QPointer<QScreen> for m_dockScreen member
variable
2. This prevents dangling pointer issues when screens are removed or
changed
3. QPointer automatically becomes null when the QScreen object is
deleted
4. Added include for QPointer header file
5. Updated copyright year range from 2023 to 2023-2026

Influence:
1. Test dock behavior when switching between different screen
configurations
2. Verify dock doesn't crash when monitors are disconnected or
reconnected
3. Test multi-monitor setup with dock moving between screens
4. Verify screen-related dock properties update correctly after screen
changes
5. Test system behavior during display configuration changes

fix: 修复屏幕切换时的野指针问题

1. 将 m_dockScreen 成员变量从 QScreen* 改为 QPointer<QScreen>
2. 防止屏幕被移除或更改时出现悬空指针问题
3. QPointer 在 QScreen 对象被删除时会自动变为空指针
4. 添加了 QPointer 头文件包含
5. 更新了版权年份范围从 2023 到 2023-2026

Influence:
1. 测试在不同屏幕配置切换时的停靠栏行为
2. 验证断开或重新连接显示器时停靠栏不会崩溃
3. 测试多显示器设置中停靠栏在屏幕间移动的情况
4. 验证屏幕更改后停靠栏的屏幕相关属性正确更新
5. 测试显示配置更改时的系统行为
@deepin-ci-robot
Copy link

deepin-ci-robot commented Feb 12, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sourcery-ai
Copy link

sourcery-ai bot commented Feb 12, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Replaces a raw QScreen* member with a QPointer in the dock panel to prevent dangling pointer crashes when screens are removed or changed, adds the corresponding include, and updates the file copyright header years.

Sequence diagram for dock screen deletion with QPointer

sequenceDiagram
    participant DockPanel
    participant QScreen
    participant QtObjectSystem

    DockPanel->>QScreen: store pointer in m_dockScreen (QPointer<QScreen>)
    QtObjectSystem-->>QScreen: delete QScreen due to screen removal
    QScreen-->>QtObjectSystem: destroyed
    QtObjectSystem-->>DockPanel: m_dockScreen automatically set to null
    DockPanel->>DockPanel: check m_dockScreen before screen-dependent logic
    DockPanel-->>DockPanel: skip or update behavior if m_dockScreen is null
Loading

Class diagram for DockPanel using QPointer for dock screen

classDiagram
    class DockPanel {
        QPointer~QScreen~ m_dockScreen
    }

    class QScreen

    DockPanel --> QScreen : references
Loading

File-Level Changes

Change Details Files
Use a guarded Qt pointer for the dock’s associated screen to avoid wild/dangling pointer access when displays change.
  • Change the dock panel member that stores the associated screen from a raw QScreen* to QPointer so it is automatically nulled when the screen is destroyed.
  • Include the QPointer header so the guarded pointer type is available in the dock panel declaration.
  • Ensure any existing code that uses m_dockScreen will now safely handle the case where the screen has been deleted instead of reading a dangling pointer.
 panels/dock/dockpanel.h
Refresh the copyright header range to include years up to 2026.
  • Update SPDX-FileCopyrightText line to use a 2023 - 2026 year range.
 panels/dock/dockpanel.h
Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@deepin-ci-robot
Copy link

deepin-ci-robot commented Feb 12, 2026

deepin pr auto review

这段代码的修改主要是将 QScreen* 类型的成员变量 m_dockScreen 改为了 QPointer<QScreen>。这是一个非常典型且正确的针对 Qt 框架中 QObject 派生类指针的安全改进。

以下是对该 diff 的详细审查意见，涵盖语法逻辑、代码质量、代码性能和代码安全四个方面：

1. 语法逻辑

  • 审查结果：通过
  • 分析：
    • 语法上完全正确。QPointer 是 Qt 提供的模板类，专门用于持有 QObject 派生类的指针。
    • QScreen 继承自 QObject，因此符合 QPointer 的使用要求。
    • 头文件引入 #include <QPointer> 也是必要的。

2. 代码质量

  • 审查结果：优秀
  • 分析：
    • 自动置空：QPointer 的行为类似于普通的 QScreen*，但有一个关键区别：当引用的对象（即 m_dockScreen 指向的屏幕对象）被销毁时，QPointer 会自动将其重置为 nullptr。
    • 避免悬空指针：在 GUI 程序中，屏幕可能会被热插拔（例如拔掉显示器）或者系统配置改变导致屏幕对象被 Qt 核心销毁。使用普通指针 QScreen* 在此时会变成“悬空指针”，如果后续代码未做判空操作直接使用，会导致程序崩溃。使用 QPointer 大大提高了代码的健壮性。

3. 代码性能

  • 审查结果：影响极小（可忽略）
  • 分析：
    • 内存开销：QPointer 相比于普通指针，内部多存储了一个 QObject 的弱引用句柄（或者类似的机制），增加了极少量（通常是一个指针大小）的内存开销。对于一个类中的成员变量来说，这完全可以忽略。
    • 访问开销：解引用 QPointer（如 m_dockScreen->geometry()）和普通指针几乎一样快，没有显著的性能损耗。
    • 构造/析构开销：当 QScreen 对象销毁时，QPointer 需要接收通知并置空，这涉及到少量的信号槽或事件处理机制。但在屏幕拔插这种低频事件下，这种性能损耗完全在可接受范围内。

4. 代码安全

  • 审查结果：显著提升
  • 分析：
    • 防止崩溃：这是本次修改的核心价值。它有效地防止了“Use-After-Free”（释放后使用）类型的内存访问错误。
    • 使用建议：虽然 QPointer 能自动置空，但在使用该成员变量时，代码中仍应保持良好的判空习惯（例如 if (m_dockScreen) { ... }），或者配合 QPointer 的 isNull() 或布尔转换检查，以确保逻辑的绝对严密。

总结与改进建议

总结：
这是一个非常高质量的修改。它利用 Qt 的智能指针机制解决了 GUI 开发中常见的生命周期管理问题，显著提升了 DockPanel 类的稳定性，特别是在处理多显示器或热插拔场景时。

改进建议：
虽然头文件修改得很好，但为了确保这一改动完全生效，建议同步检查对应的 .cpp 实现文件：

  1. 初始化检查：确保 m_dockScreen 在构造函数初始化列表中被正确初始化（默认为 nullptr）。
  2. 使用前检查：在 .cpp 文件中所有访问 m_dockScreen 的地方，确认是否已经有判空逻辑。虽然 QPointer 保证了安全，但业务逻辑上可能需要处理屏幕为空时的降级方案。
  3. 逻辑一致性：如果代码中有类似 connect(m_dockScreen, &QScreen::destroyed, ...) 的逻辑，使用 QPointer 后通常可以移除手动处理 destroyed 信号的代码，因为 QPointer 已经自动管理了这种关系，可以进一步简化代码。

sourcery-ai[bot]
sourcery-ai bot reviewed Feb 12, 2026
View reviewed changes
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@18202781743
Copy link
Contributor Author

18202781743 commented Feb 12, 2026

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sourcery-ai sourcery-ai[bot] sourcery-ai[bot] left review comments

@mhduiy mhduiy Awaiting requested review from mhduiy

@yixinshark yixinshark Awaiting requested review from yixinshark

@robertkill robertkill Awaiting requested review from robertkill

@BLumia BLumia Awaiting requested review from BLumia

@fly602 fly602 Awaiting requested review from fly602

@wjyrich wjyrich Awaiting requested review from wjyrich

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@18202781743 @deepin-ci-robot